Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the TinyMCE init array #10968

Merged
merged 5 commits into from
Nov 20, 2018
Merged

Fix the TinyMCE init array #10968

merged 5 commits into from
Nov 20, 2018

Conversation

azaozz
Copy link
Contributor

@azaozz azaozz commented Oct 23, 2018 •
edited by youknowriad
Loading

closes #10966, #11147, #7942, #8670

  • Added "manual" convert from php to js, same as in class-wp-editor.php.
  • Reformatted the code there so it's more readable.

@azaozz
Fix the TinyMCE init array
3007f1f 
- Added "manual" convert from php to js, same as in class-wp-editor.php.
- Reformated the code so it's more readable.
@azaozz azaozz requested review from aduth and ellatrix October 23, 2018 19:10
ellatrix
ellatrix reviewed Oct 24, 2018
View reviewed changes
lib/client-assets.php
'wptextpattern',
'wpview',
);
$tinymce_plugins = apply_filters( 'tiny_mce_plugins', $tinymce_plugins, 'classic-block' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why shouldn't we pass editor if we want it to be as close as possible like the old editor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is (supposedly) the editor id and is intended to identify the TinyMCE instance when more than one on the same page. The classic editor passes content there, but thinking it's no good to reuse that as the classic block editor instance is quite different and plugins should be able to identify it from php.

Passing editor is a bit too generic :)

@ellatrix
Copy link
Member

Added "manual" convert from php to js, same as in class-wp-editor.php.

Is it not possible to compose the right PHP object, then JSON encode?

@azaozz
Copy link
Contributor Author

azaozz commented Oct 24, 2018 •
edited
Loading

No, because that breaks some of the functionality. The TinyMCE init is supposed to be written in (pure) js. It is in our (WordPress) implementation that we set it as php and then "convert" to js before output.

Several settings in the init object accept js functions. If it is JSON encoded, that breaks. Example: init_instance_callback. To test can use the mini WP plugin from here: #10966 (comment).

Currently when using that plugin we get this in the console:

Uncaught TypeError: Cannot read property 'apply' of undefined
    at Editor.execCallback (tinymce.js?ver=4800-20180716)
    at initEditor (tinymce.js?ver=4800-20180716)
    .....

Happens because the init js is outputted by wp_localize_script() which is meant only for strings and JSON encodes the output.

With this pr we get:

Editor: editor-7ffd9aaf-ae66-47f1-a160-1820fe202478 is now initialized.

i.e. works as expected.

@azaozz
Copy link
Contributor Author

azaozz commented Oct 24, 2018 •
edited
Loading

Also, this pr adds another (custom) option to the TinyMCE init object: 'classic_block_editor' => true,. It would be nice to be able to identify the TinyMCE instance from js based plugins. Will add similar option to the Classic editor's instance, probably 'content_editor' => true, (or maybe that should be classic_editor?).

@azaozz
Copy link
Contributor Author

azaozz commented Oct 24, 2018 •
edited
Loading

Heh? Tests failing because... the code is more readable? :)

     |         |     expected 13 space(s) between "'toolbar1'" and
     |         |     double arrow, but found 1.

This seems contrary to the WP coding standards. So when defining arrays they should look like:

'long-array-key-that-would-make-for-a-long-line-and-there-is-no-way-to-wrap-it-up' => 'value',
'key'                                                                              => 'long-array-key-value-that-is-not-easy-to-read-as-it-makes-the-line-too-long',

The spacing looks so "borked" :) How is that easy to read? :)

Opened WordPress/WordPress-Coding-Standards#1513 as follow up.

This was referenced Oct 30, 2018
Closed
Closed
Closed
@danielbachhuber
Copy link
Member

danielbachhuber commented Oct 30, 2018 •
edited
Loading

Note: this doesn't quite yet solve #11147 (comment)

@azaozz
Copy link
Contributor Author

azaozz commented Nov 2, 2018

Also opened https://core.trac.wordpress.org/ticket/45221 for core.

This is a regression and a blocker for WP 5.0.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 8, 2018

Fixed in core https://core.trac.wordpress.org/changeset/43867. The fix needs to be merged here too.

@youknowriad
Copy link
Contributor

Is this ready to land? Can we rebase and merge @iseulde @azaozz ?

youknowriad
youknowriad approved these changes Nov 20, 2018
View reviewed changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@youknowriad
Copy link
Contributor

These changes are already in Core, so no need to backport.

@youknowriad youknowriad added this to the 4.5 milestone Nov 20, 2018
@azaozz
Copy link
Contributor Author

azaozz commented Nov 20, 2018 •
edited
Loading

Ughhh, PHPCS forcing ugly code formatting :(
(I should have fixed that, sorry).

@youknowriad youknowriad merged commit 18d0fbc into master Nov 20, 2018
@youknowriad youknowriad deleted the fix/tinymce-init-array branch November 20, 2018 17:00
@pbiron pbiron mentioned this pull request Nov 20, 2018
Closed
daniloercoli added a commit that referenced this pull request Nov 20, 2018
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
12dc2ba 
…rnmobile/wire-on-replace-para-block

* 'master' of https://github.com/WordPress/gutenberg:
  Remove "permalink settings" link from permalink panel. (#12121)
  preserve quote content (#12122)
  Replace gutenberg domain with default for Core blocks (#12108)
  Fix issue with disabled togglecontrol double border (#12091)
  Fix the TinyMCE init array (#10968)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ellatrix ellatrix ellatrix left review comments

@youknowriad youknowriad youknowriad approved these changes

@aduth aduth Awaiting requested review from aduth

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.

Fix the classic block TinyMCE init array
5 participants
@azaozz @ellatrix @danielbachhuber @youknowriad @mtias